Skip to content

Conversation

@Lokesh9106
Copy link

This commit adds a warning message in the CoreML partitioner's partition() method to alert users when they use the older to_edge() flow instead of the recommended to_edge_transform_and_lower() flow.

The warning informs users that:

  • Using the old flow may result in performance regression
  • The recommended approach is to use to_edge_transform_and_lower() with CoreML partitioner
  • Provides a link to the CoreML backend documentation for more details

Fixes #15960

This commit adds a warning message in the CoreML partitioner's partition() method to alert users when they use the older to_edge() flow instead of the recommended to_edge_transform_and_lower() flow.

The warning informs users that:
- Using the old flow may result in performance regression
- The recommended approach is to use to_edge_transform_and_lower() with CoreML partitioner
- Provides a link to the CoreML backend documentation for more details

Fixes pytorch#15960
@pytorch-bot
Copy link

pytorch-bot bot commented Nov 24, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/15963

Note: Links to docs will display an error until the docs builds have been completed.

❌ 6 New Failures

As of commit 7f967ac with merge base 93bf861 (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla
Copy link

meta-cla bot commented Nov 24, 2025

Hi @Lokesh9106!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@github-actions
Copy link

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@meta-cla
Copy link

meta-cla bot commented Nov 24, 2025

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 24, 2025
@metascroy
Copy link
Contributor

Great start! But I'm afraid adding a warning inside the partitioner itself isn't sufficient because the partitioner is also used in the to_edge_lower_and_transform flow.

Perhaps you could take a look at the similar change for XNNPACK: #13209

We should also add some unit tests showing: 1) the warning is logged with to_edge, but 2) not logged with to_edge_lower_and_transform.

@Lokesh9106
Copy link
Author

Lokesh9106 commented Nov 24, 2025 via email

@metascroy
Copy link
Contributor

Can I make changes and resubmit it

Absolutely!

Lokesh9106 and others added 3 commits November 25, 2025 08:49
… workflow

Following the pattern from PR pytorch#13209 (XNNPACK), this commit:

1. Adds `import inspect` to enable call stack inspection
2. Adds `_check_if_called_from_to_backend()` helper method that:
   - Returns False if called from to_edge_transform_and_lower (recommended flow)
   - Returns True if called from to_backend in deprecated flow
3. Wraps the deprecation warning in a conditional check

This ensures the warning only appears when using the deprecated to_edge() + to_backend() workflow, not when using the recommended to_edge_transform_and_lower() flow.

Unit tests will be added in a separate commit.
Adds two unit tests to verify the deprecation warning behavior:

1. `test_deprecation_warning_for_to_backend_workflow`: Verifies that the warning IS logged when using the deprecated to_edge() + to_backend() workflow

2. `test_no_warning_for_to_edge_transform_and_lower_workflow`: Verifies that the warning is NOT logged when using the recommended to_edge_transform_and_lower() workflow

Also adds necessary imports:
- import io
- import logging  
- from executorch.exir import to_edge, to_edge_transform_and_lower

These tests ensure the deprecation warning only appears in the deprecated flow, following the same pattern as XNNPACK (PR pytorch#13209).
@metascroy
Copy link
Contributor

Looks great! Make you you run the linter though.

To run the linter, execute the following from the executorch repo folder:

lintrunner -a

This requires you install the lintrunner (pip install -r requirements-lintrunner.txt).

# test_runner.test_symint_arg()
test_runner.test_take_over_constant_data_false()

def test_deprecation_warning_for_to_backend_workflow(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these be methods inside TestCoreMLPartitioner?

Then in the main section you can add test_runner.test_deprecation_warning_for_to_backend_workflow() and test_runner.test_no_warning_for_to_edge_transform_and_lower_workflow()

), "When lower_full_graph=True, you must set take_over_mutable_buffer=True"


def _check_if_called_from_to_backend(self) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the indentation here is off. As written, this is defined inside init

@pytorch-bot
Copy link

pytorch-bot bot commented Dec 3, 2025

To add the ciflow label ciflow/trunk please first approve the workflows that are awaiting approval (scroll to the bottom of this page).

This helps ensure we don't trigger CI on this PR until it is actually authorized to do so. Please ping one of the reviewers if you do not have access to approve and run workflows.

@pytorch-bot pytorch-bot bot removed the ciflow/trunk label Dec 3, 2025
@metascroy
Copy link
Contributor

Are you still working on this @Lokesh9106 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add warning when users use to_edge instead of to_edge_transform_and_lower with CoreML

2 participants